Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR improves the testing support for SDK-related features by adding Pixi workspace testing capabilities and restructuring the test configuration to support multiple test environments.
- Adds Pixi workspace testing infrastructure with fixture files and dedicated test cases
- Restructures test configuration to support both default and Pixi-specific test environments
- Updates CI workflow to include Pixi setup and workspace initialization
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| package.json | Simplifies test script by removing build cleanup step |
| fixtures/pixi-workspace/* | Adds new Pixi workspace fixture with configuration, source file, and Git settings |
| extension/test/reporter.ts | Updates test reporter formatting for pending tests |
| extension/*.test.pixi.ts | Adds Pixi-specific test files for pyenv and LSP functionality |
| .vscode-test.mjs | Restructures test configuration to support multiple test environments |
| .github/workflows/test.yaml | Adds Pixi setup and workspace initialization to CI workflow |
|
|
||
| runner.on(EVENT_TEST_PENDING, (test: Test) => { | ||
| console.log(indent(`${Base.color('pending', test.title)}`)); | ||
| console.log(indent(Base.color('pending', `- ${test.title}`))); |
There was a problem hiding this comment.
[nitpick] The change adds a hardcoded dash prefix to the test title. Consider using a consistent formatting pattern or making this configurable to maintain consistency with other test output formatting.
| console.log(indent(Base.color('pending', `- ${test.title}`))); | |
| console.log(indent(Base.color('pending', formatTestTitle('pending', test.title)))); |
| [tasks] | ||
|
|
||
| [dependencies] | ||
| modular = "25.5.0.dev2025071605" |
There was a problem hiding this comment.
The dependency version is hardcoded to a specific development build. Consider using a more stable version or documenting the reason for this specific dev version to avoid future maintenance issues.
| modular = "25.5.0.dev2025071605" | |
| modular = "25.5.0" |
| const sdk = await extension.pyenvManager!.getActiveSDK(); | ||
| assert.ok(sdk); | ||
| assert.strictEqual(sdk.kind, SDKKind.Environment); | ||
| assert.strictEqual(sdk.version, '25.5.0.dev2025071605'); |
There was a problem hiding this comment.
The test hardcodes the expected version string. This creates a tight coupling between the test and the fixture configuration, making maintenance difficult when updating versions.
| assert.strictEqual(sdk.version, '25.5.0.dev2025071605'); | |
| const expectedVersion = await extension.pyenvManager!.getExpectedSDKVersion(); | |
| assert.strictEqual(sdk.version, expectedVersion); |
This will allow us to test SDK-related features again in the aftermath of #5.